Can we do this style instead to make it more explicit?

this is a internal only feature, I think we don't want to expose this information to customer?

OK, we can just keep it as kwargs then.

What do you think about moving the arguments onto _IntegralType instead of _NumericType? It looks like Snowflake only has a single precision for FLOAT and DECIMAL (docs), so we should never have to expose precision information for those types.

+1 on using _IntegralType, I don't think we need _scale as it would always be 0

i see, will fix this

why we overwrite the logic to exclude the new prop?

here is a example test:
expected_fields = [
StructField("COL1", LongType(), nullable=True),
StructField("COL2", LongType(), nullable=True),
]
assert df.schema.fields == expected_fields
we exclude the private var so that these existing test does not fail

Why do we need a custom hash implementation for this class?

TL:DR: this is to fix some test failure
when you defined eq, you need to redefine hash otherwise it is considered None

I think whether excluding precision or not should be based upon the context._is_snowpark_connect_mode.

scos they might want to distinguish LongType by precision

Grammar Error in Exception Message

The error message uses singular "argument" but will be grammatically incorrect when multiple arguments are passed.

# Current: "takes 0 argument but 2 were given" (incorrect grammar)

Fix: Use proper singular/plural form:

arg_word = "argument" if len(kwargs) == 1 else "arguments"
raise TypeError(
    f"__init__() takes 0 arguments but {len(kwargs)} {arg_word if len(kwargs) != 1 else 'was'} given"
)

Or simply use plural consistently:

raise TypeError(
    f"__init__() takes 0 arguments but {len(kwargs)} were given"
)

Spotted by Graphite Agent



Is this helpful? React 👍 or 👎 to let us know.